fix(crop-area): initial render with defined objectFit for initialCrop… #551
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
I also confirmed that this change doesn't affect the fix for #506 so it's safe |
| @@ -742,7 +742,7 @@ class Cropper extends React.Component<CropperProps, State> { | |||
| classes: { containerClassName, cropAreaClassName, mediaClassName }, | |||
| } = this.props | |||
|
|
|||
| const objectFit = this.state.mediaObjectFit | |||
| const objectFit = this.state.mediaObjectFit ?? this.getObjectFit() | |||
There was a problem hiding this comment.
Could we instead add the following code inside the componentDidMount():
const objectFit = this.getObjectFit()
if (objectFit !== this.state.mediaObjectFit) {
this.setState({ mediaObjectFit: objectFit }, this.computeSizes)
}I think it would fit better with the current logic of having it stored in the state.
There was a problem hiding this comment.
I just tried it and I got the following:
Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate.
React limits the number of nested updates to prevent infinite loops.
This probably happened because calling this.computeSizes this early doesn't work. The media must be loaded before this function is called. It calculated a zoom level of NaN and everything broke from there.
I also tried to remove the "this.computeSizes" callback and this cleared the error but the initialCroppedAreaPixels didn't work. I was back to the previous issue.
I don't have time to troubleshoot further.
There was a problem hiding this comment.
Alright, let's call this good enough then :)
|
🚀 PR was released in |
Fixes: #550